Solarbank 2 Schedule/Power control#36
Conversation
- Exposes SolixBLE set_schedule(power_w) flip-dots/SolixBLE#36 as a user-facing control. - Number entity (0-800 W, 10 W step, slider) stages the target value; - Apply button to commit it to the device. - Last value restored across HA restarts via RestoreEntity. - Library errors surface as HomeAssistantError so they show as UI notifications. - No Version increment yet -> Maintainer - Add Solarbank 2 to README supported devices list - Added unit tests
| # WARNING # | ||
| # Functions below are # | ||
| # NOT E2E Tested on Device!! # | ||
| ############################## |
There was a problem hiding this comment.
Maybe lets avoid adding untested stuff for now, this PR and file is already very large and we can always add it later, ideally using the same CMD_xxx constant style used for other commands.
There was a problem hiding this comment.
That being said I don't want to waste the work already here that might be useful for someone who wants to test/refine this stuff in future but I am not sure where else to put it. Maybe we add an experimental section to the Solarbank2 docs with this layout info in it?
There was a problem hiding this comment.
Hi,
i've been running julians PR-branch for controlling my Solarbank 2 E1600 AC, as I wanted to control two options only available through the APP and can vouch for these functions at least on my E1600 AC (which is different in hw setup from the PRO) but the captures looked the same for these on my device so i tried them.
I have two additional controls added in my local branch, which are very specific to the AC ( "enable 1000w emergency power through AC socket without grid" and "charge battery from grid for X minutes" ) , which I would like to add when this PR is done
There was a problem hiding this comment.
Either way is fine for me, now that we have confirmation that it works I'd simply remove that tag. Thanks for confirming @ConniShiva
| async def _initiate_negotiations(self) -> None: | ||
| """Start the legacy base SolixBLEDevice handshake.""" | ||
| _LOGGER.info("SB2: starting legacy base handshake (00xx/08xx, AES-CBC)") | ||
| await super()._initiate_negotiations() |
There was a problem hiding this comment.
I think it makes sense to cut this since _initiate_negotiations() already produces a log entry when called.
|
|
||
| # Encryption helpers | ||
|
|
||
| def _encrypt_with_static_key(self, plaintext: bytes) -> bytes: |
There was a problem hiding this comment.
When I originally implemented the prime protocol I think I just stored the cipher text in constants instead of storing the plain text and then encrypting it before sending it since they were constants, but since you now need that functionality I think it would make more sense to implement it in the _encrypt_payload() function of prime_device.py.
In _decrypt_payload() it automatically determines if it should use the static or dynamic key depending upon if the dynamic key is set, you should be able to use that to implement something similar for _encrypt_payload()
|
|
||
| Two variants live in this module: | ||
|
|
||
| * :class:`Solarbank2` - uses the legacy base ``SolixBLEDevice`` handshake |
There was a problem hiding this comment.
I am almost tempted to say that the parts of Solarbank2Prime that might not be specific to the Solarbank2 should be in a separate file, like how I have device.py and device_prime.py so it would be idk like device_solarbank.py, but it seems a tad premature to give it its own class without knowing if its all specific to the Solarbank2 or if its shared with other Solarbank models.
I think its fine to leave for now but it might make sense to move it later, especially with how large this file has gotten.
There was a problem hiding this comment.
I had exactly the same thoughts and couldn't decide either :D The solarbank family is quite big nowadays
|
|
||
| # Post-handshake command path | ||
|
|
||
| async def _send_command(self, cmd: bytes, payload: bytes) -> None: |
There was a problem hiding this comment.
This is because the timestamp SolixBLE generates always starts at the same time isn’t it and the timestamp for Solarbanks needs to be correct for schedules to work properly right?
Long term I think it makes sense to upgrade _send_command() and the negotiation in prime_device.py so that the timestamp is in line with the actual time but I originally avoided doing that so I could keep using constants for the negotiation, it made testing easier since it would always produce the same value, and no supported devices needed accurate time, but I guess that’s not true any more.
Keeping this here is fine for now but is something I should probably fix eventually and to do that it would be good to have some tests for the negotiation of the solarbank to make sure it still works when I get around to refactoring it. There are some negotiation tests here and here which should help.
There was a problem hiding this comment.
If I recall correctly the timestamps should at least be higher than the negotiation timestamp. It's not really comparing anything to an actual realtime clock if I recall it correctly. But it's been some time and I'm not sure anymore.
Yes I think so too, but I the PR was super big already and I didn't want to refactor everything with only limited testing capabilities
| After reassembly it calls ``self._decrypt_payload``, which on this | ||
| class is PrimeDevice's AES-GCM variant. | ||
| """ | ||
| return await SolixBLEDevice._process_telemetry_packet(self, payload, cmd) |
There was a problem hiding this comment.
I think it might make sense to merge the _process_telemetry_packet of prime_device.py and device.py at some point in the future but this is fine for now as long as we have some tests for telemetry packet processing.
Looking at test_telemetry_packet_processing() I can see that there are tests for telemetry packet processing of Prime devices and some Solix power stations but none for the Solarbank2, could you add some please.
| _FAKE_SB2_USER_ID_ENV = "1" * 40 | ||
|
|
||
|
|
||
| def test_sb2_build_set_schedule_payload_matches_expected_layout(): |
There was a problem hiding this comment.
It would be good if all of these tests were parameterised like the other ones so they could test more values. I don't mind if you make up the data for them rather than using captures, the purpose of most of the tests in this module are more about making sure that changes to the code don’t result in unintended changes to the output rather than strictly adhering to observed behaviour.
| assert result.hex() == expected_hex | ||
|
|
||
|
|
||
| @pytest.mark.parametrize("bad_power", [-1, 801, 1000]) |
There was a problem hiding this comment.
Its more of a nitpick than anything but I like to use pytest.param(a, b, ..., n, id="useful_test_id") in all parameterised tests so that if a test fails there is an ID which gives me an initial indication of what the issue might be. That being said I am not super fussed about this since its kind of obvious.
So for these it might be something like "negative", "one_over", and "super_massive".
| Solarbank2Common._build_set_schedule_payload(bad_power) | ||
|
|
||
|
|
||
| def test_sb2_build_set_schedule_payload_uses_fresh_nonce_per_call(): |
There was a problem hiding this comment.
You know with how many damn devices SolixBLE now supports I think I might have to reconsider how tests are structured since this file is kind of massive. This is fine for now though.
There was a problem hiding this comment.
Yeah if this keeps growing this would be massive 😄
| // Utilities | ||
|
|
||
| function log(msg) { | ||
| console.log(msg); |
There was a problem hiding this comment.
Hmmm.
Since its already here I guess you it makes sense to use the log function from frida_2.js instead of removing it.
| :doc:`Solarbank 2 <solarbank2>`) instead perform their per-session BLE | ||
| AES inside the **Dart AOT** code in ``libapp.so`` using the | ||
| ``pointycastle`` library. The Java Cipher hooks never fire for that | ||
| traffic, so the session key cannot be recovered through them. |
There was a problem hiding this comment.
I am impressed with the lengths you have gone to update the docs, have you tested that this all renders properly? I forget the exact commands to build the docs locally but I think its something like cd docs and then make html and then in the output folder there is an index.html which you can open in a browser which should contain the local changes. You might need to install the stuff in docs/requirements.txt first.
There was a problem hiding this comment.
No worries, it's not done without the documentation ;-)
Actually I consider this part of figuring out the additional crypto libs my main contribution. The rest is just following/extending your templates with the new device knowledge.
Yes I have run the build and quickly skimmed through it. Didn't notice any obvious flaws.
I think your docu build pipeline in HA-Solix-Ble can be used to view the docs build of this PR if I recall it correctly
Edit: It's even in the auto build pipeline in this PR :-) https://solixble--36.org.readthedocs.build/en/36/
Ah dammit, looking at it I just discovered I put the wrong script name into the docs :-/
| + (1440).to_bytes(2, "little") | ||
| + power_w.to_bytes(2, "little") | ||
| + bytes.fromhex("5000") | ||
| ) |
There was a problem hiding this comment.
I think I remember seeing a better way of packing bytes using something like construct but I am not familiar with it and that kind of structural change is probably better to implement later, I think its fine as long as there are enough tests to make sure I don’t break it when refactoring, so like 3+ test calls of this with different parameters.
| .. note:: | ||
| A pristine, never-paired SB2 may still require a one-time pairing | ||
| through the Anker app before any BLE client (including SolixBLE) | ||
| can connect. |
There was a problem hiding this comment.
Apologies if this is already explained elsewhere and I just missed it, but how would one go about getting this cloud ID, does the app show it anywhere?, if you use the legacy protocol is it included in the telemetry data? or might there be some command in the legacy protocol to request it that could be found in the dumps?
There was a problem hiding this comment.
So the ideal workflow for the future is:
- We reverse engineer the pairing process (not the handshake, the actual first time pairing with the button press).
- I am 99% sure this id is transferred during pairing
- We implement our own pairing in SolixBLE
- User can set whatever ID he/she pleases
In the meantime maybe it's possible to get the ID from the cloud API, but I haven't tried because I knew mine from the captures
|
|
||
| Uses 40xx/48xx negotiation across 8 stages and AES-GCM for session | ||
| traffic. Requires an Anker user-id. SB2 firmware whitelists user-ids | ||
| and rejects unknown values with RX 4827 = ``09 a1 02 b4 00``. |
There was a problem hiding this comment.
It wouldn’t happen to accept a 0000000 user ID would it? Its probably wishful thinking but we have seen Anker take a few shortcuts when it comes to security so I wouldn’t be completely surprised.
There was a problem hiding this comment.
No it rejects a random user ID. But as I said above, reverse engineering the pairing process is probably very easy now. I simply ran out of time
| TLVs). | ||
| * Stage-6 sets the timezone (TX 4022); stage-7 re-sends the user-id | ||
| (TX 4027) session-encrypted. | ||
| * Post-stage-5 payloads use the ``fe 05 03 <ts>`` trailer rather than |
There was a problem hiding this comment.
Its interesting that it uses the timestamp format that Solix devices seem to use from device.py rather than the timestamp format used in Prime devices, it would be nice if Anker would make up their damn mind.
I suppose its probably something I should also look into to see if I can make the existing code more portable but its fine for now.
There was a problem hiding this comment.
Hm true that's interesting... Good catch!
|
|
||
| Differences from Anker Prime power stations: | ||
|
|
||
| * Per-session random ECDH private key (Prime uses a hardcoded one). |
There was a problem hiding this comment.
I suppose I should probably consider making the other implementations use dynamic ones as well. I did it that way because it made testing easier and I am not super concerned about this type of security which would require close proximity to exploit, but I suppose I should fix it... eventually...
There was a problem hiding this comment.
Yeah I'm pretty much with you here. (And Anker as well)
Whoever can hack this can probably also just cut the wire.
Claude put that nit-picky comment there and I didn't remove it. I didn't want to call you out or whatsoever. Sorry. I'll remove it.
| "it does not require an Anker user ID." | ||
| ) | ||
| anker_user_id = env_val | ||
| if isinstance(anker_user_id, str): |
There was a problem hiding this comment.
There should probably be a length check here if people need to be manually entering Anker account IDs, maybe even some basic regex to make sure its sane since I would not be surprised if people tried to use their emails in it. Some tests as well I guess.
flip-dots
left a comment
There was a problem hiding this comment.
You know I gotta hand it to you, this is some great work, its highlighted quite a few shortcomings with the existing abstractions I made which I should probably fix at some point, hell most of my review comments are just me highlighting things for future me to do.
In theory my availability should be a lot better now that my university exams are sorted so feel free to reach out if you need additional clarification or some help with this.
|
Thank you so much for taking your time to review this big blob so thoroughly. I appreciate it a lot! I will consider every one of your comments, I just replied to the obvious ones where I don't need to change much on my side. With that being said my current availability is very poor for the next few months, so I don't know when I'll find the time to work on this again. If someone else wants to pick this up in the meantime I'm fine with it and I'm happy to help. |
Changes:
405e. Currently restricted to setting one power output 24/7[untested]:Tests & Docu
Why two handshakes?
As I mentioned in the issue #28 I have implemented both handshakes/protocol variants:
Prime handshake
I included the Prime style variant because it seems like the "right" way to interact with the device and a lot of work went into it. The legacy handshake might get removed any time, so it's good to have a fallback option.
"Legacy" handshake
I included the Legacy variant because right now it is so much simpler for our use case. No env vars, no user specific config.